Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - chore(Data/Set): split Data/Set/Function #17091

Closed
wants to merge 4 commits into from

Conversation

b-mehta
Copy link
Contributor

@b-mehta b-mehta commented Sep 24, 2024

Take the lemmas about monotonicity of functions out of Data/Set/Function and move them to Data/Set/Monotone.

Also remove Compl.decidableMem, as it is already given in decidableCompl (instance search already finds it too)


Open in Gitpod

@b-mehta b-mehta added tech debt Tracking cross-cutting technical debt, see e.g. the "Technical debt counters" stream on zulip t-data Data (lists, quotients, numbers, etc) labels Sep 24, 2024
Copy link

github-actions bot commented Sep 24, 2024

PR summary aa0f8babe1

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference
There are 3764 files with changed transitive imports taking up over 161318 characters: this is too many to display!
You can run scripts/import_trans_difference.sh all locally to see the whole output.

Declarations diff

- Compl.decidableMem

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

Mathlib/Data/Set/Monotone.lean Outdated Show resolved Hide resolved
Mathlib/Data/Set/Monotone.lean Outdated Show resolved Hide resolved
@Ruben-VandeVelde Ruben-VandeVelde added the awaiting-author A reviewer has asked the author a question or requested changes label Sep 29, 2024
Mathlib/Data/Set/Monotone.lean Outdated Show resolved Hide resolved
Mathlib/Data/Set/Monotone.lean Outdated Show resolved Hide resolved
Mathlib/Data/Set/Monotone.lean Outdated Show resolved Hide resolved
Co-authored-by: Ruben Van de Velde <[email protected]>
Co-authored-by: damiano <[email protected]>
@b-mehta b-mehta removed the awaiting-author A reviewer has asked the author a question or requested changes label Sep 29, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be under Order? Order.Monotone.Set maybe?

Copy link
Contributor Author

@b-mehta b-mehta Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced by that name - a good chunk of the lemmas in Order.Monotone involve Set anyway, so it's not a good disambiguator. More specifically, what would be the deciding factor for a new lemma going in Order.Monotone.Set as opposed to any one of the other files in Order.Monotone?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's little to none, which surely is an argument for putting the files close together or even merge them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not opposed to putting the files close together, I just don't think Order.Monotone.Set is a good name for the new file!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Order.Monotone.SetFunction? 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More importantly, the lemmas in this file use lemmas from Data.Set.Function, which also creates an import cycle.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See

but right now this is not possible as `Set.preimage` is not defined yet, and importing it creates

, for instance.

Sounds like the "right now" is very outdated

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the lemmas in this file use lemmas from Data.Set.Function

Where? I don't see any lemma from Data.Set.Function being used. I see definitions being used, but those could move to Data.Set.Operations

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it is not just definitions that are used here, lemmas from Data.Set.Function are used in the file. Furthermore, even if it were only definitions being used, it is out of scope of this PR to additionally move around definitions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, but merging was merely a suggestion for how to put the material under Order.Monotone. Please move this new file under Order.Monotone

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🎉

bors merge

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the ready-to-merge This PR has been sent to bors. label Oct 4, 2024
mathlib-bors bot pushed a commit that referenced this pull request Oct 4, 2024
Take the lemmas about monotonicity of functions out of Data/Set/Function and move them to Data/Set/Monotone.

Also remove `Compl.decidableMem`, as it is already given in `decidableCompl` (instance search already finds it too)



Co-authored-by: adomani <[email protected]>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Oct 4, 2024

Build failed (retrying...):

mathlib-bors bot pushed a commit that referenced this pull request Oct 4, 2024
Take the lemmas about monotonicity of functions out of Data/Set/Function and move them to Data/Set/Monotone.

Also remove `Compl.decidableMem`, as it is already given in `decidableCompl` (instance search already finds it too)



Co-authored-by: adomani <[email protected]>
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Oct 4, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore(Data/Set): split Data/Set/Function [Merged by Bors] - chore(Data/Set): split Data/Set/Function Oct 4, 2024
@mathlib-bors mathlib-bors bot closed this Oct 4, 2024
@mathlib-bors mathlib-bors bot deleted the cut-data-set-function branch October 4, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors. t-data Data (lists, quotients, numbers, etc) tech debt Tracking cross-cutting technical debt, see e.g. the "Technical debt counters" stream on zulip
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants